-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix swiftIdentifier
crash with empty strings
#105
Conversation
@@ -114,10 +114,10 @@ enum SwiftIdentifier { | |||
static func prefixWithUnderscoreIfNeeded(string: String, | |||
forbiddenChars exceptions: String = "") -> String { | |||
|
|||
let (head, _) = identifierCharacterSets(exceptions: exceptions) | |||
guard let firstChar = string.unicodeScalars.first, | |||
!string.isEmpty else { return "" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string.isEmpty
can't ever be false if string.unicodeScalars.first
is not nil
, right? If string
is empty, then string.unocodeScalars.first
will return nil
and already make the condition fail anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'd think so, wouldn't you? Give it a try, for some absolutely bizarre reason, the isEmpty
check is needed 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait seriously?
isEmpty checks on characters, unicodeScalars is different, so that could make sense, maybe… but can't see an example
Do you mean that if we don't test isEmpty, the test on "" fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It throws an exception (SIGINT or something I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested by cloning your branch and removing that !string.isEmpty
in the condition and all test passed successfully, without SIGINT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which Xcode version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10.0 final
3fc3f51
to
772fdc3
Compare
772fdc3
to
6a981aa
Compare
Fixes SwiftGen/SwiftGen#528.